Skip to content

Compilation: fix compiler_rt and ubsan_rt strategy logic#24866

Merged
andrewrk merged 1 commit into
ziglang:masterfrom
mlugg:rt-building-logic
Aug 18, 2025
Merged

Compilation: fix compiler_rt and ubsan_rt strategy logic#24866
andrewrk merged 1 commit into
ziglang:masterfrom
mlugg:rt-building-logic

Conversation

@mlugg

@mlugg mlugg commented Aug 15, 2025

Copy link
Copy Markdown
Member

It doesn't really make sense for target_util.canBuildLibCompilerRt (and its ubsan-rt friend) to take in use_llvm, because the caller doesn't control that: they're just going to queue a sub-compilation for the runtime. The only exception to that is the ZCU strategy, where we effectively embed _ = @import("compiler_rt") into the Zig compilation: there, the question does matter. Rather than trying to do multiple weird calls to model this, just have canBuildLibCompilerRt return not just a boolean, but also differentiate the self-hosted backend being capable of building the library vs only LLVM being capable. Logic in Compilation uses that difference to decide whether to use the ZCU strategy, and also to disable the library if the compiler does not support LLVM and it is required.

Also, remove a redundant check later on, when actually queuing jobs. We've already checked that we can build compiler_rt, and compiler_rt_strat is set accordingly. I'm guessing this was there to work around a bug I saw in the old strategy assignment, where support was ignored in some cases.

Resolves: #24623

@mlugg mlugg force-pushed the rt-building-logic branch 2 times, most recently from 5e916b5 to 64fca4e Compare August 17, 2025 13:15
It doesn't really make sense for `target_util.canBuildLibCompilerRt`
(and its ubsan-rt friend) to take in `use_llvm`, because the caller
doesn't control that: they're just going to queue a sub-compilation for
the runtime. The only exception to that is the ZCU strategy, where we
effectively embed `_ = @import("compiler_rt")` into the Zig compilation:
there, the question does matter. Rather than trying to do multiple weird
calls to model this, just have `canBuildLibCompilerRt` return not just a
boolean, but also differentiate the self-hosted backend being capable of
building the library vs only LLVM being capable. Logic in `Compilation`
uses that difference to decide whether to use the ZCU strategy, and also
to disable the library if the compiler does not support LLVM and it is
required.

Also, remove a redundant check later on, when actually queuing jobs.
We've already checked that we can build `compiler_rt`, and
`compiler_rt_strat` is set accordingly. I'm guessing this was there to
work around a bug I saw in the old strategy assignment, where support
was ignored in some cases.

Resolves: ziglang#24623
@mlugg mlugg force-pushed the rt-building-logic branch from 64fca4e to fbe81bc Compare August 17, 2025 13:17
@andrewrk andrewrk merged commit c1483eb into ziglang:master Aug 18, 2025
25 of 26 checks passed
@mlugg mlugg deleted the rt-building-logic branch November 21, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wasm32-wasi-musl does not link ubsan

2 participants